Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve cancellation of large response in TransModel API #5908

Merged
merged 2 commits into from
Jun 20, 2024
Merged

Improve cancellation of large response in TransModel API #5908

merged 2 commits into from
Jun 20, 2024

Conversation

vpaturet
Copy link
Contributor

@vpaturet vpaturet commented Jun 14, 2024 •
edited
Loading

Summary

#5883 implemented support for cancelling queries that produce too large responses.
However GraphQL resolvers were not properly canceled, as in the case of an API timeout exception.
This PR applies the same logic for large response and API timeout.
Both error cases are mapped to the same HTTP error code (422 - Unprocessable Content) since the root cause is identical: a syntactically correct request that uses too much resources.

Note: multilingual error messages should be updated.

Response sent to the API client:


{
  "errors": [
    {
      "message": "The number of fields in the GraphQL result exceeds the maximum allowed: 1000000",
      "locations": [],
      "extensions": {
        "classification": "ResponseTooLarge"
      }
    }
  ]
}

Issue

No

Unit tests

Added unit tests

Documentation

No

@codecov Codecov
Copy link

codecov bot commented Jun 14, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 35.00000% with 13 lines in your changes missing coverage. Please review.

Project coverage is 69.45%. Comparing base (b9e54d2) to head (9be3805).
Report is 20 commits behind head on dev-2.x.

Files Patch % Lines
.../AbortOnUnprocessableRequestExecutionStrategy.java 0.00% 5 Missing ⚠️
...entripplanner/apis/transmodel/TransmodelGraph.java 0.00% 3 Missing ⚠️
...planner/ext/restapi/resources/PlannerResource.java 0.00% 2 Missing ⚠️
...ner/apis/transmodel/ResponseTooLargeException.java 0.00% 2 Missing ⚠️
...s/transmodel/MaxFieldsInResultInstrumentation.java 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@              Coverage Diff              @@
##             dev-2.x    #5908      +/-   ##
=============================================
- Coverage      69.45%   69.45%   -0.01%     
- Complexity     17064    17069       +5     
=============================================
  Files           1927     1929       +2     
  Lines          73578    73584       +6     
  Branches        7549     7550       +1     
=============================================
+ Hits           51106    51107       +1     
- Misses         19847    19852       +5     
  Partials        2625     2625

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@vpaturet vpaturet force-pushed the fix_too_large_response_cancellation branch from f81fa32 to 44c79dc Compare June 14, 2024 07:55
@vpaturet vpaturet marked this pull request as ready for review June 14, 2024 08:00
@vpaturet vpaturet requested a review from a team as a code owner June 14, 2024 08:00
@vpaturet vpaturet requested a review from t2gran June 14, 2024 08:01
@vpaturet vpaturet self-assigned this Jun 14, 2024
@vpaturet vpaturet added Improvement Entur Test This is currently being tested at Entur labels Jun 14, 2024
@leonardehrenfried leonardehrenfried self-requested a review June 18, 2024 09:00
t2gran
t2gran previously approved these changes Jun 19, 2024
View reviewed changes
leonardehrenfried
leonardehrenfried requested changes Jun 19, 2024
View reviewed changes
Copy link
Member

@leonardehrenfried leonardehrenfried left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few translation changes.

@vpaturet vpaturet merged commit 69191b5 into opentripplanner:dev-2.x Jun 20, 2024
5 checks passed
t2gran pushed a commit that referenced this pull request Jun 20, 2024
Add changelog entry for #5908 [ci skip]
aa7cdd9
@vpaturet vpaturet deleted the fix_too_large_response_cancellation branch June 24, 2024 10:35
@t2gran t2gran added this to the 2.6 (next release) milestone Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@t2gran t2gran t2gran approved these changes

@leonardehrenfried leonardehrenfried leonardehrenfried approved these changes

Assignees

@vpaturet vpaturet

Labels
Entur Test This is currently being tested at Entur Improvement
Projects
None yet
Milestone
2.6
Development

Successfully merging this pull request may close these issues.
3 participants
@vpaturet @leonardehrenfried @t2gran